ci(windows): gate packaged sandbox lifecycle evidence - #3558
Conversation
22c7ad6 to
3ce3d4d
Compare
|
Could a maintainer rerun the failed The failure occurred in the existing Evidence for rerun rather than a scope-expanding fix here:
The contributor token cannot rerun upstream Actions ( |
3ce3d4d to
2862abf
Compare
Generated-by: Codex
2862abf to
229d09c
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Review at exact head 229d09c727f4719c70b01e5e975023ffc09408a6.
The gate itself is sound. Both PowerShell verifiers are awaited in sequence and any non-zero exit rejects through runCommand, so missing or malformed lifecycle evidence fails closed rather than passing vacuously — the failure mode that makes a gate decorative. I confirmed the exact-head hosted run 32634441543 (job 97181975711) really executed the lifecycle step, and the evidence it gates on comes from that same fresh package: there is no path by which a stale artifact from an earlier run could satisfy it.
[P1] The path allowlist doesn't cover the sources the gate actually exercises
The new trigger entries reach the packaged worker and the Windows boundary driver:
- 'packages/runtime/src/filesystem-worker/**'
- 'packages/runtime/src/sandbox/**'
- 'packages/core/src/permission-profile.ts'
- 'packages/core/src/permission-profile-compiler.ts'But the code in those directories imports Core modules that are not listed. Directly:
packages/core/src/sandbox-boundary.ts→ imported byfilesystem-worker/client.ts,operations.ts,protocol.tspackages/core/src/windows-path.ts→ imported bysandbox/windows-profile.ts,sandbox/windows-sandbox.ts
and one level further, sandbox-boundary.ts imports serialized-byte-length.js while windows-path.ts imports absolute-path.js. The Runtime side has the same gap for child-fd-input.ts, child-process-lifecycle.ts and process-tree-terminator.ts.
From Core, only the two permission-profile* files are listed. So a change confined to sandbox-boundary.ts — the module that defines the boundary this gate exists to protect — triggers neither pre-merge Windows lane. The W0 PR filter doesn't cover it either. A sandbox-boundary regression would land on main and surface only in the nightly, which is precisely the "found at the worst possible moment" situation the workflow header says this path was built to prevent.
This is worth raising because the PR is otherwise strictly an improvement; the gap is that the allowlist is a hand-copied prefix of the real dependency closure, and hand-copied closures drift the moment someone adds an import.
Suggested direction: rather than appending these seven files, derive the trigger set from the actual import graph of the packaged E2E entrypoints, and add a contract test asserting that every source reachable from those entrypoints appears in the workflow's paths. That converts "someone remembered to update the list" into a checked property, and it fails loudly when a new import escapes the gate. If deriving it is too large for this PR, adding the seven files plus that contract test would still close the drift.
Nothing else found. Verification: exact-head test and package are completed/success; 65/65 targeted tests plus a fake-rejection probe covering missing and malformed second-stage evidence. Windows packaging, PowerShell, the Rust broker, AppContainer/ACL, installer/update/rollback and the full repo suite were not run locally and are not claimed.
2 participants
Summary
Promotes existing Windows sandbox lifecycle evidence into the shared packaged verifier used by both the pull-request
Release Windows checkand the formal release workflow.The verifier runs
appcontainer-smoke.ps1andacl-recovery-smoke.ps1against the exact broker underwin-unpacked. Those existing probes cover concurrent AppContainer launches, process/Job drain, residual ACE and ledger cleanup, and recovery after the broker is killed mid-launch. The pull-request path filter includes the packaged worker and Windows boundary source closure so relevant changes cannot skip this gate.This changes release verification only. It does not modify Runtime, broker, launch, cancellation, retry, ACL, or sandbox policy behavior.
Refs #2142
Remaining Phase 4 work
This PR intentionally does not close the Phase 4 lifecycle checkbox. Explicit client cancellation, Runtime Host death during launch, sustained concurrency soak, unsettled-state recovery/quarantine, the wider adversarial matrix, and independent human security review remain separate follow-up gates.
Verification
Exact head
229d09c72, rebased without conflicts ontomain6e5e7562eafter #3265 and the upstream ASF-header fix:node --test scripts/ci-test-plan.test.mjs scripts/verify-windows-harness.test.mjs: 64 passed, 0 failed, 1 privilege-dependent symlink skip.biome lint .: 2,611 files, no findings.biome format .: 1,600 files, no changes.git diff --checkpassed.range-diffconfirms the rebase preserves this PR's implementation; the only harness context change is feat(win): Abort-path installer rollback with backup retention #3265's independent registry-ownership test, and both tests pass together.L3 on the exact head:
AI use
Tool(s) and scope: Codex traced the existing Windows evidence and release ownership, implemented the shared packaged gate and trigger contract, rebased it after #3265, retained both independent harness contracts, ran the local quality gate, and performed an author-side adversarial review. The commit includes a
Generated-by: Codextrailer. AI review is not independent human security review.Checklist
Does this PR entail a change in behavior?